Conversation
This will save a bit of memory and make instance construction faster.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
There are some cases that may be not correct.
| Graph: _TypeAlias = dict[str, "State"] | ||
|
|
||
|
|
||
| @mypyc_attr(acyclic=True) |
There was a problem hiding this comment.
I don't think this is true, because of direct_dependents that points back to SCC that depends on the current one. This is used to notify dependents when they are ready to be processed.
There was a problem hiding this comment.
Actually ignore this one, we refer to direct dependents by id, so we should be good here.
| return isinstance(other, UnpackType) and self.type == other.type | ||
|
|
||
|
|
||
| @mypyc_attr(acyclic=True) |
There was a problem hiding this comment.
I am not sure we can actually guarantee this. There is source_any that can refer to itself because of a flaw in type inference logic somewhere.
| COMPLETE_FEATURES: Final = frozenset((TYPE_VAR_TUPLE, UNPACK, NEW_GENERIC_SYNTAX)) | ||
|
|
||
|
|
||
| @mypyc_attr(acyclic=True) |
There was a problem hiding this comment.
Again, _per_module_cache can cause problems in theory. Not sure how dangerous it is.
|
I'll review the potential issues and revert anything that I think could realistically be a problem. |
hauntsaninja
left a comment
hauntsaninja
left a comment
There was a problem hiding this comment.
(would be cool if mypy was able to validate use of this to some extent)
I'm not sure if we can validate this very reliably, since even self-references might not imply a cycle (it could be a tree), and even an attribute of type |
3 participants
This will save a bit of memory and make instance construction faster. The impact will be hard to measure, possibly under the noise floor, but it should be a slight improvement, and acts as a real-world test case of the new mypyc feature.